Variable length connection IDs #1879
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thynson
reviewed
May 23, 2024
| @@ -561,22 +558,6 @@ impl Endpoint { | |||
| .. | |||
| } = incoming.packet.header; | |||
|
|
|||
| if self.cids_exhausted() { | |||
There was a problem hiding this comment.
One option is, changing return type of the ConnectionIdGenerator.generate_cid to Result<ConnectionId, SomeError>, so that if the implementation of ConnectionIdGenerator is willing to perform exhausted check, it could return an Err(CidExhausted). But since it must be a breaking change, I'm not sure if it's worth to do it in this way.
There was a problem hiding this comment.
These changes are breaking regardless, so we might as well take the cleanest approach. However:
- The generator would need to know the total number of live CIDs, requiring additional complexity
- Large deployments might have a difficult time tracking the number of CIDs in use in a shared space across multiple endpoints
- It's not clear if this check is all that useful to begin with
I'm presently leaning towards a simpler approach that just gives up if it can't generate a unique CID after some number of tries, to at least handle the case where someone is using an extremely small CID space for some reason.
Ralith
force-pushed
the
variable-length-cids
branch
from
7479835 to
2757df1
Compare
Ralith
force-pushed
the
variable-length-cids
branch
6 times, most recently
from
077fa06 to
5ceba42
Compare
Allows generators to be shared with connections, and removes an obstacle to parallelizing endpoint work in the future.
Now that CID generators can have shared ownership, there's no need to indirect through a factory function in the config.
We can't track this if the size of the CID space isn't readily known, and we already implicitly rely on `new_cid` being infallible.
Ralith
force-pushed
the
variable-length-cids
branch
from
5ceba42 to
615a1ed
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a breaking change with no pressing demand.